Skip to content

feat: add GroupColumn support for Float16 in multi-column GROUP BY#23785

Open
tohuya6 wants to merge 1 commit into
apache:mainfrom
tohuya6:feat-22715-float16-group-column
Open

feat: add GroupColumn support for Float16 in multi-column GROUP BY#23785
tohuya6 wants to merge 1 commit into
apache:mainfrom
tohuya6:feat-22715-float16-group-column

Conversation

@tohuya6

@tohuya6 tohuya6 commented Jul 22, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

multi_group_by::group_column_supported_type gates which GROUP BY columns may
use the column-wise GroupValuesColumn fast path, and the gate is
all-or-nothing: a single unsupported column forces the entire grouping onto
the byte-encoded GroupValuesRows fallback, even when every other key column
would have qualified. A Float16 key triggers exactly that today.

Float16 reuses the existing PrimitiveGroupValueBuilder with no new builder
type: its native half::f16 already implements the HashValue
canonicalization (hash_float!(f16, f32, f64)), so -0.0/+0.0 folding and
NaN grouping match Float32/Float64 with no extra handling.

What changes are included in this PR?

  • Accept Float16 in group_column_supported_type and dispatch it in
    make_group_column.
  • Move Float16 from the rejected to the accepted set in the
    group_column_supported_typemake_group_column consistency fuzz, and
    repoint the two tests that used Float16 as their stock "unsupported" example
    to a permanently-invalid unit combo (Time64(Second)), so they stay stable as
    sibling primitive builders (Decimal256, Interval, …) land independently.
  • Add a (Float16, Int32) group-count benchmark to benches/multi_group_by.rs
    (capped below f16's ~63.5k distinct finite values).

Are these changes tested?

Yes.

  • New unit test test_group_values_column_float16: a Float16 key stays on the
    GroupValuesColumn path, dedups including nulls, folds -0.0/+0.0 into one
    group (stored as +0.0), groups equal NaNs, and round-trips with the
    Float16 output type preserved.
  • The consistency fuzz now asserts Float16 routes through the dispatcher.
  • New single- and multi-column Float16 GROUP BY coverage in aggregate.slt.

Are there any user-facing changes?

No API changes. GROUP BY queries with a Float16 key now use the column-wise
fast path instead of the row-encoded fallback; results are unchanged.

…umn GROUP BY

`multi_group_by::group_column_supported_type` gates which GROUP BY columns can
use the column-wise `GroupValuesColumn` fast path. Any unsupported column forces
the entire grouping onto the byte-encoded `GroupValuesRows` fallback, so a single
`Float16` key dragged an otherwise-qualifying multi-column GROUP BY onto the
slow path.

`Float16` reuses the existing `PrimitiveGroupValueBuilder` with no new builder
type: its native `half::f16` already implements the `HashValue` canonicalization
(`hash_float!(f16, f32, f64)`), so `-0.0` / `+0.0` folding and NaN grouping
match Float32 / Float64 with no extra handling.

- accept `Float16` in `group_column_supported_type` and dispatch it in
  `make_group_column`
- move `Float16` from the rejected to the accepted set in the
  `group_column_supported_type` <-> `make_group_column` consistency fuzz, and
  repoint the two tests that used `Float16` as their stock "unsupported" example
  to a permanently-invalid unit combo (`Time64(Second)`), so they stay stable as
  sibling primitive builders (Decimal256, Interval, ...) land
- add an end-to-end unit test (Float16 GROUP BY dedups including nulls, folds
  -0.0/+0.0 into one group stored as +0.0, groups equal NaNs, and preserves the
  Float16 output type) and a Float16 GROUP BY block in aggregate.slt
- add a `(Float16, Int32)` group-count benchmark to `benches/multi_group_by.rs`
  (capped below f16's ~63.5k distinct finite values)

Part of apache#22715
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant